-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bump request@2.83.0 #2103
Bump request@2.83.0 #2103
Conversation
Updating request due to tough-cookie security fix More info: request/request#2776
Unfortunately this patch would effectively break node < 4 support.
…On 28 Sep. 2017 1:43 am, "James Foster" ***@***.***> wrote:
Updating request due to tough-cookie security fix
More info: request/request#2776
<request/request#2776>
------------------------------
You can view, comment on, or merge this pull request online at:
#2103
Commit Summary
- Bump ***@***.***
File Changes
- *M* package.json
<https://github.com/sass/node-sass/pull/2103/files#diff-0> (2)
Patch Links:
- https://github.com/sass/node-sass/pull/2103.patch
- https://github.com/sass/node-sass/pull/2103.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#2103>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAjZWLfHhIzVaVt7YhaekaPq7l9rvIsbks5smm0ygaJpZM4Pl-09>
.
|
Understood. I'll close this out. |
Hey, first off, love node-sass and have used it for years, thanks for all the hard work you all do. That being said, I'm a little alarmed that using node-sass will now be introducing potential vulnerabilities to my application. My understanding is that the vulnerability is not that serious with a small attack surface. Node-sass only uses the request package to download binary files during the installation phase and no-where else. As such, the attack vector would require compromising the url destination and injecting malicious data into the cookie causing the regex slowdown. Potentially this could make npm install fail, however, if the the attacker has compromised the server hosting the binaries they could make the install fail regardless, e.g. by returning null. Can anyone confirm/deny this? If I've made any mistakes please let me know. :) Nonetheless in order to be compliant we have a strict 0 vulnerabilities policy. Even though this vulnerability seems very minor we still must fix it. The only solution I can think of is forking and breaking node < 4 support. Any comments? Thanks again, |
The |
Thanks for the response, you're entirely correct. For the record, it turns out that I'm getting different results if I use npm install $npm list tough-cookie
npm info it worked if it ends with ok
npm info using npm@5.3.0
npm info using node@v8.6.0
project@1.1.0 /PROJECTDIR/
└─┬ node-sass@3.13.1
└─┬ request@2.83.0
└── tough-cookie@2.3.3
npm info ok Then in between, I yarn project@1.1.0 /PROJECTDIR/
├─┬ karma@1.4.1
│ └─┬ chokidar@1.7.0
│ └─┬ fsevents@1.1.2
│ └─┬ node-pre-gyp@0.6.36
│ └─┬ request@2.81.0
│ └── tough-cookie@2.3.2 # PROBLEM!
└─┬ node-sass@3.13.1
└─┬ request@2.83.0
└── tough-cookie@2.3.3 Anyways, thanks again. I'll be digging deeper and maybe opening an issue for Yarn if I can get a test case. |
Updating request due to tough-cookie security fix
More info: request/request#2776